Implement span.anchor in panel to replace id by jonahtanjz · Pull Request #1609 · MarkBind/markbind

jonahtanjz · 2021-06-28T11:40:52Z

What is the purpose of this pull request?

Overview of changes:
Follow-up to #1603 (comment). The support of scroll-margin-top on ios is still limited thus require a different approach to handle anchor navigation for panels.

Replace existing panel id with span.anchor (similar to how normal headers navigation works). This prevents the panels from getting blocked by the fixed-header bar as well as resolves #1601.

Anything you'd like to highlight / discuss:
If there is a better workaround.

Apparently changing the width of ::before does not fix #1601 as the entire panel block still occupies the vertical space of ::before as well.

The container which holds ::before still has the width of the panel

Testing instructions:

Create panel with header

<panel header="# Test">
</panel>

Click on panel link in page-nav
Panel should appear below the fixed-header
Can refer to User Guide: Unable to open details panel on mobile #1601 issue, visit nav bar section in UG, details panel should be clickable now

Proposed commit message: (wrap lines at 72 characters)
Implement span.anchor in panel to replace id

The support of scroll-margin-top on ios is still limited hence
would require a different approach to handle anchor navigation
for panels while keeping elements before panels from getting
blocked.

Let's introduce span.anchor into panel, similar to how the headers
navigation works, to support anchor navigation as well as to prevent
previous elements from getting blocked.

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No blatantly unrelated changes
Pinged someone for a review!

ang-zeyu · 2021-06-29T14:05:05Z

 import panelBase from './PanelBase';

 export default {
+  inheritAttrs: false,


hmm what's this for?

Vue passes non-prop attributes such as id to the root node of a component. In this case, we do not want the main panel container, div or span, to have the id as this interferes with anchor navigation.
The inheriAttrs: false is needed to prevent the root node of minimalPanel and nestedPanel from inheriting the id attribute.

https://v3.vuejs.org/guide/component-attrs.html#disabling-attribute-inheritance

Thanks for clarifying! Hmm how about changing the property name under assignPanelId (then using it as a prop)?

So there's no risk of side effects from disabling attribute inheritance entirely.

Yup that would work as well. Will update it.

jonahtanjz · 2021-07-02T15:57:57Z

@ang-zeyu Updated the PR to pass in panelId as prop.

ang-zeyu · 2021-07-05T11:46:26Z

Lgtm 👍, just this portion that would have to be edited also since we aren't using ::before anymore.

(handles the auto scroll back to top)

// To enable behaviour of auto window scrolling during panel collapse
if (this.$el.getBoundingClientRect().top < 0) {
  jQuery('html').animate({
    scrollTop: window.scrollY + this.$el.getBoundingClientRect().top - 3,
  }, 500, 'swing');
}

jonahtanjz · 2021-07-05T13:29:06Z

@ang-zeyu Thanks for the catch. Have updated the PR :)

ang-zeyu · 2021-07-08T12:58:32Z

        requestAnimationFrame(() => {
          // To enable behaviour of auto window scrolling during panel collapse
          if (this.$el.getBoundingClientRect().top < 0) {
+            const headerHeight = jQuery('header[fixed]').height() || 0;


a really minor nit for keeping everything under the panel implementation -- would this work?

const el = $refs.anchor || this.$el; el .getBoundingClientRect......

Hmm added ref="anchor" to the spans and doesn't seem to be working... The panel header still gets hidden under the navbar after collapsing.

try adding display: inline-block to the scoped anchor class here

Slightly better but half of it is still under the navbar... When lower navbar is present, it gets hidden again 😅

Looks like the sibling after span.anchor is interfering 🤣... let's leave it for now and revisit later

ang-zeyu

Lgtm 👍

ang-zeyu reviewed Jun 29, 2021

View reviewed changes

jonahtanjz force-pushed the Fix-blocking-panel-element branch from 24c81ba to 35357dd Compare July 3, 2021 03:52

jonahtanjz and others added 7 commits July 5, 2021 20:51

Shift id in panel div container to span.anchor

78af0de

Add test for span.anchor in panel

3ae0e03

Remove card-container css and its css logic

a849691

Update id to panelId

9bffe46

Update expected files

55090df

Rebase and resolve conflicts

782f056

Update panel auto scroll

69ef766

jonahtanjz force-pushed the Fix-blocking-panel-element branch from 35357dd to 69ef766 Compare July 5, 2021 13:26

ang-zeyu reviewed Jul 8, 2021

View reviewed changes

ang-zeyu approved these changes Jul 8, 2021

View reviewed changes

ang-zeyu added this to the v3.0.5 milestone Jul 8, 2021

ang-zeyu merged commit a07107f into MarkBind:master Jul 8, 2021

Uh oh!

Conversation

jonahtanjz commented Jun 28, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahtanjz commented Jul 2, 2021

Uh oh!

ang-zeyu commented Jul 5, 2021

Uh oh!

jonahtanjz commented Jul 5, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonahtanjz Jul 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ang-zeyu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jonahtanjz commented Jun 28, 2021 •

edited

Loading

jonahtanjz Jul 8, 2021 •

edited

Loading